feat(dashboard): replace the fixed Cycle Time sentence with a computed, honest verdict#110
Open
marcos-sabatino-clickbus wants to merge 3 commits into
Open
feat(dashboard): replace the fixed Cycle Time sentence with a computed, honest verdict#110marcos-sabatino-clickbus wants to merge 3 commits into
marcos-sabatino-clickbus wants to merge 3 commits into
Conversation
Replace the hardcoded "lead-time bottlenecks live before/after, not in code execution" sentence in the Cycle Time card with a verdict computed from the tenant's own flow data. The engine already emits per-PR phase medians (time_in_phase_median_hours) and the per-repo page renders them, but the org dashboard still showed a fixed editorial string. This aggregates those phases org-wide (weighted by merged PRs), picks the dominant phase, and renders a stacked phase bar. The verdict only ever describes the code window (PR open -> merge); it never claims anything about the "before" (demand) or "after" (deploy) windows the engine does not measure here. - summarizeFlow / selectCycleTimeVerdict: pure, fully unit-tested (100% lines, ~98% branches on the new module) - coverage-gated: below 60% the card shows a partial sample, not a verdict - aggregated by repo/org only, never per person - no DB migration, no new ingestion, no engine change Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rdict An adversarial self-review of the first commit found four accuracy defects in the "honest verdict" card. This fixes all four. M1 - share denominator: relabel "% of cycle time" -> "% of measured phase time" and document that the window total is a SUM of phase medians, not the cycle-time median (a separate open->merge aggregation shown alongside). M2 - coding contamination: the dominant phase and its share are now computed over WINDOW_PHASES (the 4 post-open phases). `coding` (first commit -> PR open) is pre-window authoring time and can no longer be reported as the bottleneck of a card that measures only PR open->merge. The phase bar shows the window phases only. M3 - coverage guard-rail was inert: flowCoveragePct used pr_merged_count as both numerator and denominator (always ~100%). The engine now emits flow_pr_count (the real decomposed-PR count; pr_count already existed on FlowEfficiencyResult), and the platform weights/measures coverage by it. Payloads without flow_pr_count are skipped so coverage never fakes 100%. M4 - removed the redundant "(wait)/(espera)" tag, which only ever appended to phase labels that already said "wait/espera". Tests rewritten for the new semantics (weight, window dominant, real coverage) with cases locking M2 (coding never wins) and M3 (coverage < 1 and null without flow_pr_count). TS 231 pass, Python 239 pass, tsc clean, cycle-time-flow.ts 100% lines / 97.5% branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ctive Follow-up from the post-fix re-review, which found one new major (a UI consistency regression that the M3 coverage fix made routine) plus the orphaned isWait field. - FlowPhaseBar now renders only when verdict.variant === "verdict". Before, it showed in lowCoverage and none too (dominantPhase is set in every variant), so an affirmative phase bar appeared next to a "partial sample, not a verdict" banner — and below the density floor with no banner at all. Honest coverage (M3) made lowCoverage common, so this was about to ship as a systematic contradiction. - Re-expose the wait/active signal lost when M4 dropped the waitTag: the phase bar now hatches wait-phase segments (via WAIT_PHASES) with a short legend note, instead of a redundant text tag. The orphaned DominantPhase.isWait field (its only reader was the removed waitTag) is deleted; tests assert wait-ness via WAIT_PHASES directly. TS 231 pass, tsc clean, cycle-time-flow.ts 100% lines / 97.5% branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
The Cycle Time card showed a hardcoded editorial sentence — "lead-time bottlenecks live before (demand/product) and after (infra, environments, deploy) — not in code execution" — rendered identically for every tenant, not derived from any data. This replaces it with a verdict computed from the tenant's own flow data, and surfaces the per-phase decomposition the engine already produced but the org dashboard discarded.
The verdict only ever describes the window it measures (PR open → merge). It makes no claim about the "before" (demand) or "after" (deploy) windows the engine doesn't measure here.
How
time_in_phase_median_hours) the engine already emits into an org-level decomposition (summarizeFlow), weighted by the real count of decomposed PRs.WINDOW_PHASES) —coding(pre-PR authoring) never wins.flow_pr_count— the honest coverage numerator (pr_countalready existed onFlowEfficiencyResult, it just wasn't persisted).Commits
feat— compute the verdict + phase bar from data already in the payload.fix— four accuracy majors from an adversarial review: share-denominator wording, exclude pre-PRcoding, make the coverage guard-rail actually live (flow_pr_count), drop a redundant wait tag.fix— gate the phase bar to the affirmativeverdictvariant (a UI consistency regression the coverage fix exposed) and re-express wait/active visually.Verification
tsc --noEmit: clean · ESLint: clean.lib/queries/cycle-time-flow.ts): 100% lines / 97.5% branches / 100% functions.flow_pr_count).Deploy note
Real coverage requires repos re-analyzed with the new engine so
flow_pr_countreaches the payload. Until then, older payloads →flow = null→ the card shows a neutral state with no causal claim. Graceful by design.Non-blocking follow-ups
buildVerdictText,FlowPhaseBar) isn't unit-tested — vitest runs innode(no jsdom) in this repo.Implements opportunity E1 from the platform evaluation; hardened through a review → fix → re-review loop.
🤖 Generated with Claude Code